Skip to content

Optimise SSZ encoding and decoding #55

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 24 commits into from
Nov 12, 2025
Merged

Optimise SSZ encoding and decoding #55

merged 24 commits into from
Nov 12, 2025

Conversation

@michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Aug 28, 2025 •
edited
Loading

Summary of changes

  • Add unsafe fast path for decoding lists and vectors of u8
  • Optimise generic decoding by using chunks_exact and an explicit loop instead of try_fold.

Benchmarks

fixed_vector/decode_u8_1m
                        time:   [18.182 µs 18.272 µs 18.372 µs]
                        change: [−99.716% −99.714% −99.711%] (p = 0.00 < 0.05)
                        Performance has improved.
fixed_vector/encode_u8_1m
                        time:   [944.36 µs 951.38 µs 956.95 µs]
                        change: [−4.5123% −3.9002% −3.3534%] (p = 0.00 < 0.05)
                        Performance has improved.
fixed_vector/decode_u64_128k
                        time:   [73.696 µs 73.893 µs 74.086 µs]
                        change: [−59.191% −59.108% −59.033%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 19 outliers among 100 measurements (19.00%)
  19 (19.00%) low mild
fixed_vector/encode_u64_128k
                        time:   [62.911 µs 62.956 µs 63.008 µs]
                        change: [−0.4140% −0.0405% +0.3166%] (p = 0.84 > 0.05)
                        No change in performance detected.
Found 22 outliers among 100 measurements (22.00%)
  22 (22.00%) high severe

variable_list/decode_u8_1m
                        time:   [18.328 µs 18.342 µs 18.357 µs]
                        change: [−99.733% −99.733% −99.732%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe
variable_list/encode_u8_1m
                        time:   [1.0898 ms 1.0904 ms 1.0910 ms]
                        change: [+6.4182% +6.5683% +6.6894%] (p = 0.00 < 0.05)
                        Performance has regressed.
variable_list/decode_u64_128k
                        time:   [72.604 µs 72.631 µs 72.660 µs]
                        change: [−57.304% −57.272% −57.241%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
variable_list/encode_u64_128k
                        time:   [80.341 µs 80.351 µs 80.362 µs]
                        change: [+7.6504% +8.2800% +8.9044%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 11 outliers among 100 measurements (11.00%)
  1 (1.00%) low severe
  3 (3.00%) low mild
  6 (6.00%) high mild
  1 (1.00%) high severe

Additional Info

Depends on a new release of ethereum_ssz for:

Encoding is not meaningfully faster, but would need to be optimised in ssz itself (we need to optimise the Vec impl).

@codecov
Copy link

codecov bot commented Aug 28, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 60.00000% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.10%. Comparing base (8e5b6df) to head (dad52cb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/variable_list.rs 58.33% 10 Missing ⚠️
src/fixed_vector.rs 61.90% 8 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #55      +/-   ##
==========================================
- Coverage   57.18%   57.10%   -0.09%     
==========================================
  Files          10       10              
  Lines         313      345      +32     
==========================================
+ Hits          179      197      +18     
- Misses        134      148      +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This was referenced Aug 28, 2025
Open
Merged
@michaelsproul michaelsproul changed the title Optimise FixedVector Decode Optimise SSZ encoding and decoding Sep 4, 2025
@michaelsproul michaelsproul mentioned this pull request Sep 4, 2025
Draft
1 task
michaelsproul
michaelsproul commented Sep 4, 2025
View reviewed changes
michaelsproul
michaelsproul commented Sep 4, 2025
View reviewed changes
michaelsproul
michaelsproul commented Sep 4, 2025
View reviewed changes
michaelsproul
michaelsproul commented Sep 4, 2025
View reviewed changes
@michaelsproul michaelsproul mentioned this pull request Sep 8, 2025
Merged
@michaelsproul michaelsproul mentioned this pull request Sep 9, 2025
Merged
michaelsproul
michaelsproul commented Sep 10, 2025
View reviewed changes
@michaelsproul michaelsproul mentioned this pull request Sep 10, 2025
Closed
@michaelsproul michaelsproul mentioned this pull request Sep 30, 2025
Closed
4 tasks
@macladson macladson mentioned this pull request Oct 21, 2025
Merged
@michaelsproul
Copy link
Member Author

michaelsproul commented Oct 29, 2025

Unfortunately yes the bool case can trigger undefined behaviour. I've added a test in:

Miri flags it as UB and the test also fails (we decode integers like 2 "successfully" as bools)

@michaelsproul
Copy link
Member Author

michaelsproul commented Oct 29, 2025

Gonna try using Any again, which will be a breaking change.

@michaelsproul
Copy link
Member Author

michaelsproul commented Oct 29, 2025 •
edited
Loading

Ok nice, we didn't actually need an Any bound on the Decode impl, just a 'static bound. I think this is not too prohibitive, as variable lists of types containing references (i.e. non-'static) are rare. And by adding the bound only on Decode, we are just limiting users from decoding bytes into references, which doesn't really make sense anyway. E.g. doing VariableList::<&'a Struct, N>>::from_ssz_bytes(bytes) doesn't make any sense because there's nowhere for the Struct to be allocated where we can safely take a reference to it (it could be allocated on the stack, but this would violate Rust's borrowing rules).

@michaelsproul
Copy link
Member Author

michaelsproul commented Oct 29, 2025

Updated the benchmark results in the OP, there are some slower benches for encoding, but I think this is just noise. We haven't changed the encoding codepath at all.

paulhauner
paulhauner reviewed Nov 10, 2025
View reviewed changes
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the unsafe stuff looks good to me 👍

I don't understand the chunks to chunks_exact change, but perhaps I'm missing something.

I think this is overall a worthwhile improvement and the added unsafe code is (a) small enough to be easily reasoned about and (b) worth reasoning about.

@michaelsproul @paulhauner
Fix test name
c2f7140 
Co-authored-by: Paul Hauner <[email protected]>
@macladson macladson mentioned this pull request Nov 11, 2025
Open
@michaelsproul michaelsproul merged commit 89f90fd into main Nov 12, 2025
9 checks passed
@michaelsproul michaelsproul deleted the optimise-decode branch November 12, 2025 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paulhauner paulhauner paulhauner approved these changes

@macladson macladson Awaiting requested review from macladson

+1 more reviewer

@ltitanb ltitanb ltitanb left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@michaelsproul @paulhauner @ltitanb